FIX: cursor.execute() with reset_cursor=False raises Invalid cursor state #521
Merged
FIX: cursor.execute() with reset_cursor=False raises Invalid cursor state #521
Conversation
…tate #506 Add SqlHandle::close_cursor() which calls SQLFreeStmt(SQL_CLOSE) to close the ODBC cursor without destroying the prepared statement handle. When reset_cursor=False, execute() now calls close_cursor() instead of skipping cleanup entirely, allowing the prepared plan to be reused. Added 5 tests covering reset_cursor=False scenarios.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 1363-1382 1363 }
1364
1365 void SqlHandle::close_cursor() {
1366 if (_type != SQL_HANDLE_STMT || !_handle) {
! 1367 return;
! 1368 }
1369 if (_implicitly_freed) {
! 1370 return;
! 1371 }
1372 if (!SQLFreeStmt_ptr) {
! 1373 ThrowStdException("SQLFreeStmt function not loaded");
! 1374 }
1375 SQLRETURN ret = SQLFreeStmt_ptr(_handle, SQL_CLOSE);
1376 if (ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO) {
! 1377 ThrowStdException("SQLFreeStmt(SQL_CLOSE) failed");
! 1378 }
1379 }
1380
1381 SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) {
1382 if (!SQLGetTypeInfo_ptr) {📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.4%
mssql_python.pybind.connection.connection.cpp: 75.8%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%🔗 Quick Links
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes Invalid cursor state errors when re-executing statements with reset_cursor=False by closing only the ODBC cursor (not the prepared statement), enabling safe prepared-plan reuse across executions.
Changes:
- Add
SqlHandle::close_cursor()(ODBCSQLFreeStmt(SQL_CLOSE)) and expose it via Pybind11. - Update
cursor.execute()to close the ODBC cursor whenreset_cursor=False. - Add regression tests covering re-execution, multi-iteration reuse, no-params execution, and re-execution without fully consuming prior results.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_004_cursor.py | Adds regression tests validating reset_cursor=False prepared-plan reuse and cursor state handling. |
| mssql_python/pybind/ddbc_bindings.h | Declares SqlHandle::close_cursor() API. |
| mssql_python/pybind/ddbc_bindings.cpp | Implements close_cursor() and exposes it to Python via Pybind11. |
| mssql_python/cursor.py | Calls hstmt.close_cursor() when reset_cursor=False to prevent invalid cursor state on re-exec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bewithgaurav
requested changes
Apr 20, 2026
Collaborator
bewithgaurav
left a comment
bewithgaurav
left a comment
There was a problem hiding this comment.
we're proposing to revamp the full reset cursor logic in our upcoming perf PRs
the complete hstmt deletion and creation should eventually be discarded & we will move to cursor resetting by default - considering the scope of this PR, this looks good.
requesting a change to add a safety guard for implicit freeing post conn deletion.
else lgtm
… jahnvi/reset_cursor_error
…rd, hide from public API - Throw when SQLFreeStmt_ptr is not loaded instead of silent no-op - Check SQLRETURN and throw on failure (allow SQL_SUCCESS_WITH_INFO) - Add _implicitly_freed guard to prevent use-after-free - Rename to _close_cursor (underscore prefix) to mark as internal - Add assert row is not None before indexing in test
sumitmsft
approved these changes
Apr 21, 2026
bewithgaurav
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
This pull request improves the handling of cursor state when executing prepared statements with
reset_cursor=False, ensuring that the prepared plan is correctly reused and the cursor state is properly managed between executions. It introduces a newclose_cursormethod at the ODBC statement handle level, updates the Python bindings, and adds comprehensive tests to verify the new behavior.Enhancements to cursor state management:
cursor.pyto callhstmt.close_cursor()(which issues an ODBCSQLFreeStmt(SQL_CLOSE)) whenreset_cursor=False, allowing prepared statements to be reused safely without resetting the entire cursor state. This resolves issues with invalid cursor state on re-execution.ODBC handle and Python bindings updates:
close_cursormethod in theSqlHandleC++ class, which closes only the cursor on the statement handle without freeing the prepared statement.close_cursormethod to Python via theddbc_bindingsPybind11 module, allowing it to be called from Python code.Testing improvements:
test_004_cursor.pyto verify thatreset_cursor=Falsecorrectly reuses prepared plans, returns new results, works across multiple iterations, handles queries without parameters, and functions correctly when the previous result set was not fully consumed.